-
Notifications
You must be signed in to change notification settings - Fork 839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
recognize from and to block parameters in eth subscribe #826
Conversation
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @lucassaldanha may want to take a quick pass to make sure we don't break the new privacy filtering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT
.map(blockHash -> blockchain.matchingLogs(blockHash, filter.getLogsQuery())) | ||
.orElseGet( | ||
() -> { | ||
final long fromBlockNumber = filter.getFromBlock().getNumber().orElse(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should update this to be the headBlockNumber as well. This seems to be the "default" for Geth APIs. And probably update the PrivGetLogs
as well.
I'm happy to do this in another PR is you don't wanna mix up with the other changes! :)
https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_getlogs
PR description
Geth sneakily accepts from and to blocks in its websocket log subscriptions parameters. This extends the websocket subscription to be able to take block parameters. That makes us compatible with their implementation and also our http and ws filter parameters align.
Fixed Issue(s)
fixes #654